-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Download and use 16-bit SkiFree for NE tests #90
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Only one small detail: Maybe we want to add some canonical directory to the .gitignore if one wants to keep the files there locally for testing? We could also change the CI scripts to download them there. I'd still keep the option to specify the files by path if one wants to keep them separate.
Good idea! I'll look into that and see if we can add it here. It would make adding more sample binaries easier. Should that directory just be under |
How about |
This second part is bigger than the original change so it probably needs a new review. Most of the time was spent wrangling with the CI actions trying to get the downloading and caching right. The ultimate goal is to avoid downloading whenever possible, and that part is definitely working: each binary dependency now has its own reusable workflow file with a cache. We now also check the hash for the files just because we can. actions/cache requires the The default location for the binary samples is |
Resolves #3. We sort of landed on using existing binaries to test the binary image processing rather than concocting examples ourselves, at least for now.
SkiFree needs no introduction and the EXE is small. To test locally you can get the 16-bit version from Chris Pirih's site, from archive.org, or some old floppy disk you have on hand.
This fixes a bug with NE header unpacking and adds some rudimentary tests. I didn't expand the functionality yet because I'm not sure how we should handle 16-bit addresses.
We cache the download but the cache is not shared between branches so maybe using archive.org is not the best thing long-term. As we add more sample binaries we may want to parameterize the CI task and the way we load the files using
pytest
. Is that something we should figure out now or later?